Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: keep selector list in dist output of not pseudo-class #2740

Merged
merged 1 commit into from
May 10, 2024

Conversation

jawinn
Copy link
Collaborator

@jawinn jawinn commented May 8, 2024

Description

A change in the build configuration had resulted in some regressions due to increased specificity created in the dist output of :not selectors. One issue was with a disabled Menu item showing a change in icon color on hover.

The postcss-preset-env config override being removed here sets it to the default stage 2 configuration that allows multiple comma separated selectors within :not(). The configuration had been using a polyfill that converted each item in the :not selector list into its own :not selector, increasing specificity for the rule for each additional item.

Example:
Selector in dist CSS before this change: 
.spectrum-Menu-item:hover > .spectrum-Menu-itemIcon:not(.spectrum-Menu-chevron):not(.spectrum-Menu-checkmark)

Selector in dist CSS after this change (and that was generated in previous Menu 6.1.5). This is how the not selector is written in the source index.css: 
.spectrum-Menu-item:hover>.spectrum-Menu-itemIcon:not(.spectrum-Menu-chevron,.spectrum-Menu-checkmark)

CSS-755

How and where has this been tested?

Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.

Validation steps

  • Menu docs for "With disabled item(s)" no longer shows the hover color changes (as seen in screenshot)
  • The built dist/index.css is no longer creating a chain of :not(.class1):not(.class2) when defined as :not(.class, .class2). Tip: run yarn compare locally to see the dist changes in the 7 affected components.

Regression testing

Validate:

  1. The documentation pages for at least two other components are still loading, including:
  • The pages render correctly, are accessible, and are responsive.
  1. If components have been modified, VRTs have been run on this branch:
  • VRTs have been run and looked at.
  • Any VRT changes have been accepted (by reviewer and/or PR author), or there are no changes.

Screenshots

Bug noticed on the current state of Menu docs in main. When hovering over the disabled "Share link", the color of the icon changed:
Screenshot 2024-05-08 at 12 50 16 PM

To-do list

Copy link

changeset-bot bot commented May 8, 2024

🦋 Changeset detected

Latest commit: 8980ebb

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
@spectrum-css/calendar Patch
@spectrum-css/combobox Patch
@spectrum-css/datepicker Patch
@spectrum-css/menu Patch
@spectrum-css/picker Patch
@spectrum-css/slider Patch
@spectrum-css/stepper Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented May 8, 2024

File metrics

Summary

Total size: 4.48 MB*
Total change (Δ): ⬇ 1.39 KB (-0.03%)

Table reports on changes to a package's main file. Other changes can be found in the collapsed Details section below.

Package Size Δ
calendar 19.50 KB ⬇ 0.01 KB
combobox 26.31 KB ⬇ 0.19 KB
datepicker 14.51 KB ⬇ 0.04 KB
menu 40.80 KB ⬇ 0.03 KB
picker 30.64 KB ⬇ 0.03 KB
slider 32.74 KB ⬇ 0.06 KB
stepper 20.33 KB ⬇ < 0.01 KB

Details

calendar

File Head Base Δ
index-base.css 19.50 KB 19.50 KB ⬇ 0.01 KB (-0.04%)
index-vars.css 19.50 KB 19.50 KB ⬇ 0.01 KB (-0.04%)
index.css 19.50 KB 19.50 KB ⬇ 0.01 KB (-0.04%)
metadata.json 9.82 KB 9.83 KB ⬇ 0.01 KB (-0.08%)

combobox

File Head Base Δ
index-base.css 25.51 KB 25.70 KB ⬇ 0.19 KB (-0.73%)
index-theme.css 1.37 KB 1.37 KB -
index-vars.css 26.31 KB 26.49 KB ⬇ 0.19 KB (-0.71%)
index.css 26.31 KB 26.49 KB ⬇ 0.19 KB (-0.71%)
metadata.json 12.69 KB 12.88 KB ⬇ 0.19 KB (-1.46%)
themes/express.css 1.00 KB 1.00 KB -
themes/spectrum.css 0.99 KB 0.99 KB -

datepicker

File Head Base Δ
index-base.css 14.31 KB 14.35 KB ⬇ 0.04 KB (-0.25%)
index-theme.css 0.80 KB 0.80 KB -
index-vars.css 14.51 KB 14.55 KB ⬇ 0.04 KB (-0.24%)
index.css 14.51 KB 14.55 KB ⬇ 0.04 KB (-0.24%)
metadata.json 7.93 KB 7.96 KB ⬇ 0.04 KB (-0.44%)
themes/express.css 0.70 KB 0.70 KB -
themes/spectrum.css 0.69 KB 0.69 KB -

menu

File Head Base Δ
index-base.css 40.80 KB 40.83 KB ⬇ 0.03 KB (-0.07%)
index-vars.css 40.80 KB 40.83 KB ⬇ 0.03 KB (-0.07%)
index.css 40.80 KB 40.83 KB ⬇ 0.03 KB (-0.07%)
metadata.json 18.76 KB 18.79 KB ⬇ 0.03 KB (-0.15%)

picker

File Head Base Δ
index-base.css 28.73 KB 28.76 KB ⬇ 0.03 KB (-0.11%)
index-theme.css 2.49 KB 2.49 KB -
index-vars.css 30.64 KB 30.67 KB ⬇ 0.03 KB (-0.10%)
index.css 30.64 KB 30.67 KB ⬇ 0.03 KB (-0.10%)
metadata.json 14.75 KB 14.78 KB ⬇ 0.03 KB (-0.21%)
themes/express.css 1.49 KB 1.49 KB -
themes/spectrum.css 1.58 KB 1.58 KB -

slider

File Head Base Δ
index-base.css 30.40 KB 30.45 KB ⬇ 0.06 KB (-0.18%)
index-theme.css 2.92 KB 2.92 KB -
index-vars.css 32.74 KB 32.79 KB ⬇ 0.06 KB (-0.17%)
index.css 32.74 KB 32.79 KB ⬇ 0.06 KB (-0.17%)
metadata.json 14.06 KB 14.12 KB ⬇ 0.06 KB (-0.39%)
themes/express.css 1.76 KB 1.76 KB -
themes/spectrum.css 1.73 KB 1.73 KB -

stepper

File Head Base Δ
index-base.css 16.89 KB 16.89 KB ⬇ < 0.01 KB (-0.02%)
index-theme.css 4.02 KB 4.02 KB -
index-vars.css 20.33 KB 20.33 KB ⬇ < 0.01 KB (-0.02%)
index.css 20.33 KB 20.33 KB ⬇ < 0.01 KB (-0.02%)
metadata.json 9.07 KB 9.07 KB ⬇ < 0.01 KB (-0.04%)
themes/express.css 2.32 KB 2.32 KB -
themes/spectrum.css 2.28 KB 2.28 KB -
* Size determined by adding together the size of the main file for all packages in the library.
* Results are not gzipped or minified.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

Copy link
Contributor

github-actions bot commented May 8, 2024

🚀 Deployed on https://pr-2740--spectrum-css.netlify.app

@jawinn jawinn force-pushed the jawinn/css-755-not-selector-dist-format branch from 0204d7e to 52cc8fa Compare May 8, 2024 21:15
A change in configuration had resulted in some regressions due to
increased specificity created in the dist output of :not selectors.
One issue was with a disabled Menu item showing a change in icon color
on hover.

The config override being removed here sets it back to the default
stage 2 configuration that allows multiple comma separated selectors
within :not(). The configuration had been converting each item in the
:not selector list into its own :not selector, increasing specificity
for each additional item.
@jawinn jawinn force-pushed the jawinn/css-755-not-selector-dist-format branch from 52cc8fa to 8980ebb Compare May 8, 2024 21:16
Copy link
Collaborator

@rise-erpelding rise-erpelding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This solves the hover icon issue and appears to be updating those selectors with :not() correctly, nice work! 🎉

@castastrophe castastrophe merged commit c0dd6a4 into main May 10, 2024
11 checks passed
@castastrophe castastrophe deleted the jawinn/css-755-not-selector-dist-format branch May 10, 2024 20:04
@github-actions github-actions bot mentioned this pull request May 10, 2024
rise-erpelding pushed a commit that referenced this pull request Jun 1, 2024
A change in configuration had resulted in some regressions due to
increased specificity created in the dist output of :not selectors.
One issue was with a disabled Menu item showing a change in icon color
on hover.

The config override being removed here sets it back to the default
stage 2 configuration that allows multiple comma separated selectors
within :not(). The configuration had been converting each item in the
:not selector list into its own :not selector, increasing specificity
for each additional item.
rise-erpelding pushed a commit that referenced this pull request Jun 4, 2024
A change in configuration had resulted in some regressions due to
increased specificity created in the dist output of :not selectors.
One issue was with a disabled Menu item showing a change in icon color
on hover.

The config override being removed here sets it back to the default
stage 2 configuration that allows multiple comma separated selectors
within :not(). The configuration had been converting each item in the
:not selector list into its own :not selector, increasing specificity
for each additional item.
rise-erpelding pushed a commit that referenced this pull request Jun 4, 2024
A change in configuration had resulted in some regressions due to
increased specificity created in the dist output of :not selectors.
One issue was with a disabled Menu item showing a change in icon color
on hover.

The config override being removed here sets it back to the default
stage 2 configuration that allows multiple comma separated selectors
within :not(). The configuration had been converting each item in the
:not selector list into its own :not selector, increasing specificity
for each additional item.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants